-
Notifications
You must be signed in to change notification settings - Fork 81
Support react-router v6 #26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev-5.0
Are you sure you want to change the base?
Conversation
de2226d to
bc33c24
Compare
| <Routes> | ||
| <Route path='/login' element={<CustomLogin />}/> | ||
| <Route path='/sessionToken-login' element={<SessionTokenLogin />}/> | ||
| <SecureRoute path='/protected' element={<Protected />}/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to specify /protected/* here as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm, just noticed it was exact before
92b6178 to
d03728b
Compare
8ff98c4 to
e8c73b1
Compare
| @@ -1,3 +1,13 @@ | |||
| # 5.0.0 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's base this PR against the dev-5.0 branch instead of master.
| // Redirect to the /login page that has a CustomLoginComponent | ||
| // This example is specific to React-Router | ||
| history.push('/login'); | ||
| navigate('/login'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no change here. navigate should only be a replacement of restoreOriginalUri from oktaAuth to decouple react-router dependency in <Security>.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
react-router v6 has no useHistory hook, but added useNavigate instead
https://github.com/ReactTraining/react-router/blob/dev/docs/advanced-guides/migrating-5-to-6.md#use-navigate-instead-of-history
|
@denysoblohin-okta Let's split For the decoupling PR, please base it against |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shuowu
I included adding navigate prop to Security in this PR to not break support of react-router 5.
react-router 5 should use useHistory hook, react-router 6 should use useNavigate hook.
So looks like this PR depends from decoupling PR.
| // Redirect to the /login page that has a CustomLoginComponent | ||
| // This example is specific to React-Router | ||
| history.push('/login'); | ||
| navigate('/login'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
react-router v6 has no useHistory hook, but added useNavigate instead
https://github.com/ReactTraining/react-router/blob/dev/docs/advanced-guides/migrating-5-to-6.md#use-navigate-instead-of-history
Let's focus on the decoupling PR first (targeting to dev-5.0). Then get back to this PR when react-router 6 is ready. As we don't know a clear timeline for when react-router@6 will be officially available, it may even come to okta-react v6 or later. |
8331378 to
b75892c
Compare
| const { oktaAuth, authState, _onAuthRequired } = useOktaAuth(); | ||
| const match = useRouteMatch(routeProps); | ||
| const { path, caseSensitive } = routeProps; | ||
| const match = path ? useMatch.call(null, { path, caseSensitive }) : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useMatch doesn't seem to be matching a route when basename property is set
we could provide useLocation().pathname value(which includes basename) as path parameter for useMatch()
d448ab2 to
e7925e5
Compare
|
Is there a way we can merge this? React router now has a stable v6 release. Thanks! |
|
What’s the way forward here? Anything missing? |
|
Is there any news with this? |
|
Wanted to follow up on this as well. Curious if this is planned to be merged in anytime soon. |
|
Could you please merge and release this pull requests ? we need it for our deployment... |
|
@denysoblohin-okta @oleksandrpravosudko-okta Please get this merged, it's really blocking our transition to react-router-dom v6.x. Thank you. |
|
@denysoblohin-okta @oleksandrpravosudko-okta |
|
Hi there, anything new with this? May I somehow help to push it forward? Thanks. |
|
I have the same issue and I am stuck as well. This issue is important and urgent - altough react-router-dom v6 is not new anymore. |
|
wen react-router v6? |
Yes |
|
Is there any way to use this using npm? |
|
Attempting to npm install the pull request fails with a typescript error: npm install https://github.com/okta/okta-react#OKTA-318565-react-router-6 |
|
I have a v6 only version here: https://github.com/flying-sheep/okta-react/tree/flying-sheep/okta-react-router-6 see here for the changes: master...flying-sheep:flying-sheep/okta-react-router-6 |
|
This approach works for us with latest react and react router |
|
Hi guys, any update on this? is there anything missing? |
|
Any updates?? |
|
Any updates on this?? |
I created my own security route |
react-router v6 introduces breaking changes
useRouteMatchis not exported, see issue. react-router v6 usesuseMatchinstead<Route>s should be inside<Routes><Route exact>removed, so need to use*inpathif it's not exact<Route component>and<Route render>removed, replaced with<Route element>Issue: https://github.com/okta/okta-oidc-js/issues/853
Internal ref: https://oktainc.atlassian.net/browse/OKTA-318565
PR for samples: okta/samples-js-react#143
Warning! This PR uses
react-router 6.0.0-beta.0and should be revised after stable RR release. Do not merge.